Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Histogram UI fixes #363

Merged
merged 12 commits into from
May 3, 2023
Merged

Histogram UI fixes #363

merged 12 commits into from
May 3, 2023

Conversation

slvrtrn
Copy link
Contributor

@slvrtrn slvrtrn commented Apr 19, 2023

  • Support filter/filter out logs view actions
  • Fix undefined database name by default
  • Reset level and time field properly on table/database change
  • Make it possible to clear the level field (so the histogram will render without grouping by level)
  • Propagate UI changes from modifyQuery to QueryBuilder component
  • Fix filter value that gets stuck in the UI

@slvrtrn slvrtrn requested a review from a team as a code owner April 19, 2023 16:20
@slvrtrn slvrtrn requested review from asimpson and aangelisc April 19, 2023 16:20
@slvrtrn
Copy link
Contributor Author

slvrtrn commented Apr 19, 2023

The last bullet refers to a bug with a stable reproduction scenario, for example:

  • we filter out level by some values (magnifier -), let's say level != debug AND level != info
  • we filter level by some new value (magnifier +), let's say error, so it should be level = error
  • in the UI, either debug or info will be stuck like level = debug (while actually being level = error under the hood), while everything is correct in the actual QueryBuilder object, and the histogram is displayed correctly as well. The culprit is likely here, as the value in the state becomes stale, trying to find a workaround atm.

@bossinc
Copy link
Collaborator

bossinc commented Apr 27, 2023

@slvrtrn Are you referring to the magnifier between the time selector and run query?
image

I'm not able to reproduce this. What data set are you using and what queries?

@mshustov
Copy link
Collaborator

@bossinc I believe Serge referred to the magnifier in Logs panel in Explore
2023-04-28_10-15-52

@slvrtrn
Copy link
Contributor Author

slvrtrn commented Apr 28, 2023

@bossinc, as @mshustov correctly suggested, it was about the magnifiers (add a filter or filter out) in the logs panel in Explore.

@mshustov
Copy link
Collaborator

The culprit is likely here, as the value in the state becomes stale, trying to find a workaround atm.

@slvrtrn 3132f4f is supposed to fix the problem, PTAL

@slvrtrn
Copy link
Contributor Author

slvrtrn commented Apr 28, 2023

@mshustov, thanks for the fix. It is working as expected!

@bossinc, I think this one is ready for the final review and merging.

Copy link
Collaborator

@bossinc bossinc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! It just needs to be updated with main

@slvrtrn
Copy link
Contributor Author

slvrtrn commented Apr 30, 2023

@bossinc, updated

@bossinc
Copy link
Collaborator

bossinc commented May 1, 2023

@slvrtrn I'll merge this next!

@mshustov
Copy link
Collaborator

mshustov commented May 3, 2023

@bossinc can we merge now?

@bossinc bossinc merged commit cb837de into grafana:main May 3, 2023
@slvrtrn
Copy link
Contributor Author

slvrtrn commented May 4, 2023

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants